fix(plugin-auth): collapse the three basePath derivations onto one normalisation chain by os-trump · Pull Request #17002 · objectstack-ai/objectstack

os-trump · 2026-09-09T01:25:51Z

Clause-②: no

AuthManager derived its base path in three independent places — getBasePath(),
getAuthIssuer() and getMcpResourceUrl() — each reading this.config.basePath
and normalising it its own way. This lands one chain, so the count cannot grow,
and repairs the one derivation that produced a value that is not a URL.

⚠️ The card's second defect is not repaired here, because it does not exist on
main. The measurement is below; the half of the card that stays open is named at
the end.

Anchors, re-derived by text (⛔ not inherited)

Triage cited :5570 / :5902 / :5912. On the tree this branch was cut from
(7cd587413) all three had drifted 120 lines:

member	triage said	actually at	after this PR
`getBasePath()`	5570	5690	5710
`getAuthIssuer()`	5902	6022	6050
`getMcpResourceUrl()`	5912	6032	6079

validAudiences is gone, as the dispatch said: zero hits repo-wide outside three
deliberate do-not-reintroduce tombstones. The current consumer list for
getMcpResourceUrl(), re-derived by grep:

consumer	what it does with the value
`auth-manager.ts` `resources: [...]`	seeds the `sys_oauth_resource` row at plugin init — persisted
`auth-manager.ts` `clientRegistrationDefaultResources: [...]`	links every DCR client to that row — persisted
`auth-manager.ts` `getMcpProtectedResourceMetadata()`	the RFC 9728 `resource` field
`auth-manager.ts` `verifyMcpAccessToken()`	jose `audience:` — the `aud` a token is matched against
`auth-plugin.ts`	`new URL(...).pathname`, to mount the RFC 9728 §3.1 well-known route
`runtime/src/domains/mcp.ts`	the resource URL the MCP endpoint advertises
`spec/src/contracts/auth-service.ts`	declares it on `IAuthService`

Defect 1 — the value was not a URL, and nothing downstream tolerated it

Measured on a real AuthManager at 7cd587413, baseUrl: 'http://localhost:3000':

basePath 'api/v1/auth'    getMcpResourceUrl() -> http://localhost:3000api/v1/mcp    INVALID
basePath 'api/v1/auth/'   getMcpResourceUrl() -> http://localhost:3000api/v1/mcp    INVALID

Three independent confirmations that this is not an alternative spelling:

new URL('http://localhost:3000api/v1/mcp') throws Invalid URL — 3000api
is not a port. So auth-plugin.ts's new URL(manager.getMcpResourceUrl()).pathname
throws too, at discovery-route registration.
@better-auth/oauth-provider 1.7.2 refuses it at plugin init, in its own words,
observed on a real instance built by createAuthInstance:
oauth-provider: skipping resource seed for http://localhost:3000api/v1/mcp — resource identifier http://localhost:3000api/v1/mcp must be an absolute URI (RFC 8707 §2)
With the row never seeded and enforcePerClientResources at its true default,
every MCP client is refused for want of a link row.

⇒ under that configuration no token could ever have been minted or matched, so
repairing it re-selects nothing. This is triage's ruling, now with the runtime's own
refusal message behind it.

Defect 2 — ⛔ the premise is falsified; this PR pins that it stays that way

The card and its triage both read PR #16380 as having created a one-sided divergence:
better-auth handed the stripped base path while getAuthIssuer() broadcast the
retained one. That is not what landed.

git log -S 'basePath: this.getBasePath()' -- auth-manager.ts returns nothing:
that expression was never in the tree.
fix(hono): mount /auth where the auth service serves, and refuse a prefix it cannot serve under #16380's final commit is titled "hand better-auth the configured basePath verbatim
again", and its message records why: normalising the handed string made this
manager's own verifier reject every token its AS mints
(ERR_JWT_CLAIM_VALIDATION_FAILED: unexpected "iss" claim value), fail-closed and
permanent, for a trailing-slash basePath.
createAuthInstance passes configuredBasePath() — the configured value verbatim.

Measured here, reading better-auth's own (await auth.$context).baseURL — the value
@better-auth/oauth-provider 1.7.2 stamps as the access-token iss — off a real
instance, against getAuthIssuer():

configured `basePath`	better-auth `ctx.baseURL` (the `iss`)	`getAuthIssuer()`	agree?
`/api/v1/auth`	`http://localhost:3000/api/v1/auth`	same	✅
`api/v1/auth`	`http://localhost:3000/api/v1/auth`	same	✅
`/api/v1/auth/`	`http://localhost:3000/api/v1/auth/`	same	✅
`api/v1/auth/`	`http://localhost:3000/api/v1/auth/`	same	✅
`/api/v1/auth///`	`http://localhost:3000/api/v1/auth///`	same	✅
`/api/v9/identity/`	`http://localhost:3000/api/v9/identity/`	same	✅

⇒ there is no divergence to align. "Align the broadcast to the value the AS is
using right now" evaluates to change nothing, and canonicalising getAuthIssuer()
would create the fail-closed break #16380 measured and reverted. A new case pins
that equality against the real instance, so a later "canonicalisation" turns red
instead of shipping.

The single normaliser, and the sites that read it

One read of this.config.basePath remains in the file, and one chain above it:

configuredBasePath()   the configured value VERBATIM — what better-auth is handed
  └─ rootedBasePath()  + a leading slash when absent  (better-auth's own rule)
       ├─ getAuthIssuer()      = origin + this                    (published `iss`)
       └─ getBasePath()        = this, trailing slashes stripped  (mount path)
            └─ getMcpResourceUrl()  = origin + this minus `/auth` + `/mcp`

rootedBasePath() is private and is the only place a leading slash is added;
getBasePath() is the only place a trailing one is stripped (rootedBasePath() at :5628). A fourth normaliser
cannot be added without deleting a link of that chain. The two surviving values still
differ, deliberately: an issuer must mirror what better-auth stamps, trailing
slash and all; a mount path and a resource URL must be canonical.

What moves, and what provably does not

configured `basePath`	`getMcpResourceUrl()` before	after
unset · `''` · `/api/v1/auth`	`http://localhost:3000/api/v1/mcp`	unchanged
`/api/v1/auth/`	`http://localhost:3000/api/v1/mcp`	unchanged
`/api/v9/identity`	`http://localhost:3000/api/v9/identity/mcp`	unchanged
`api/v1/auth`	`http://localhost:3000api/v1/mcp` (not a URL)	`http://localhost:3000/api/v1/mcp`
`api/v1/auth/`	`http://localhost:3000api/v1/mcp` (not a URL)	`http://localhost:3000/api/v1/mcp`
`/api/v1/auth///`	`http://localhost:3000/api/v1/auth////mcp`	`http://localhost:3000/api/v1/mcp`
`/`	`http://localhost:3000//mcp`	`http://localhost:3000/mcp`

getBasePath() and getAuthIssuer() are byte-identical to before on every row.

Verification

Table-driven over every spelling, asserting the property that failed rather
than a literal: the values are constructed with new URL(...) and allowed to throw.
⛔ A hand-written literal would have re-encoded the bug.
Negative control: a canonical basePath answers byte-for-byte its current
values on all three getters — the existing canonical-only suite
(auth-manager.mcp-oauth.test.ts) is untouched and green.
Ablation, from the committed state, restore under trap ... EXIT INT TERM:
reverting getMcpResourceUrl() to the pre-fix derivation moved the on-disk blob
d791d62b → 04b4d9bf (anchor greps 1→0 and 0→1), turned the 5 new cases RED,
and left the canonical-input control GREEN. Restored: git diff HEAD empty and
the blob back to d791d62b.
Suites: @objectstack/plugin-auth 106 files / 2222 tests green; typecheck green
(after building its own dist — the first run's TS2307 was a missing prerequisite,
not a finding); @objectstack/verify auth-base-path-contract.test.ts green;
@objectstack/runtime http-dispatcher.mcp.test.ts green.
Gates: the derived union reconciled 58 derived / 58 run / 0 NOT-MEASURED /
0 UNRUN, on a tree fetched and merged to origin/main fe2b7554a.
check:dual-build-cjs-loads and check:type-check-debt first exited 3
(PREREQUISITE NOT MET, not a pass); after a full workspace build both returned a
real 0. check:route-envelope run explicitly: green (my diff writes no
c.json / res.json). pnpm lint green.

Acceptance notes

⚠️ Commit 6e0e4e87f carries a Refs #16399 trailer in its body. It was written
and pushed before the PM's addendum forbidding card-relation trailers in commit
messages arrived. Per that addendum I have not amended, rebased or force-pushed,
and have opened no replacement PR. check:partof-closing-keyword RULE 2 will red on
it; it is advisory and outside the required-context set. The merge commit and this
body are clean.
The card's "not measured — whether any deployment writes basePath non-canonically"
was deliberately not used to narrow anything. Noted, not filed.
basePath: '/' yielding a //mcp resource path that no mount serves was inside the
same derivation and is repaired with it. Noted, not filed.
[finding] /api/v1/auth is declared as two independent literals — AuthPlugin's constructor default and AuthManager's fallback — and a divergence between them is silent #16384 is untouched — a different pair of /api/v1/auth literals.

What stays open on #16399

The card's defect 2. Not because it is unrepaired, but because it was not real:
this PR delivers the measurement that falsifies it plus the pin that keeps it
falsified. #16399 is deliberately not closed by this merge so the PM can read that
finding and decide whether the card is discharged or whether the card body and the
triage comment want a correction. The consolidation and defect 1 are complete.

Generated by Claude Code

`AuthManager` derived its base path in three independent places — `getBasePath()`, `getAuthIssuer()` and `getMcpResourceUrl()` — each reading `this.config.basePath` and normalising it its own way. One of them built a value that is not a URL. `getMcpResourceUrl()` added no leading slash, so a `basePath` configured without one produced `http://localhost:3000api/v1/mcp`. Measured: `new URL()` throws on it (`3000api` is not a port), so `auth-plugin.ts`'s `new URL(manager.getMcpResourceUrl()).pathname` — which mounts the RFC 9728 §3.1 path-inserted well-known route — throws too, and `@better-auth/oauth-provider` 1.7.2 refuses it at plugin init: oauth-provider: skipping resource seed for http://localhost:3000api/v1/mcp — resource identifier ... must be an absolute URI (RFC 8707 §2) Post-#16780 that identifier seeds the `sys_oauth_resource` row (`resources`) and links every DCR client to it (`clientRegistrationDefaultResources`), with `enforcePerClientResources` at its `true` default — so the row is never written and every MCP client is refused. That input class could never mint or match a token, which is why repairing it re-selects nothing. There is now exactly ONE read of `this.config.basePath` in the file and one chain above it: configuredBasePath() the configured value VERBATIM — what better-auth is handed └─ rootedBasePath() + a leading slash when absent (better-auth's own rule) ├─ getAuthIssuer() = origin + this └─ getBasePath() = this, trailing slashes stripped └─ getMcpResourceUrl() = origin + this minus `/auth` + `/mcp` A fourth normaliser cannot be added without deleting a link of that chain. ⛔ `getAuthIssuer()` is NOT canonicalised, and the card's second defect is not repaired here because it does not exist on `main`. The card and its triage both read PR #16380 as having handed better-auth the STRIPPED base path while `getAuthIssuer()` broadcast the RETAINED one. #16380's last commit ("hand better-auth the configured basePath verbatim again") reverted exactly that, having measured that it rejects every token minted under a trailing-slash `basePath`. Measured here on a real `betterAuth()`, reading `(await auth.$context).baseURL` — the value the oauth-provider stamps as `iss` — `getAuthIssuer()` already equals it for all six spellings probed. A new case pins that equality against the real instance, so the divergence cannot be introduced by a later "canonicalisation". `getAuthIssuer()` and `getBasePath()` are byte-identical to before for every spelling. Only `getMcpResourceUrl()` moves, and only for a non-canonical `basePath`. The canonical-input control asserts all three getters unchanged. Refs #16399 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

…sepath-normaliser-consolidation

github-actions · 2026-09-09T01:26:29Z

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 6 documentable anchor(s).

11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

content/docs/api/index.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/deployment/cli.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/deployment/self-hosting.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/deployment/tenancy-modes.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/getting-started/your-first-project.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/kernel/contracts/auth-service.mdx (via AuthManager (symbol, a top-level class), getMcpResourceUrl (symbol, a method of class AuthManager))
content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class), /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class), /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/permissions/sso.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/protocol/kernel/http-protocol.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/protocol/objectui/actions.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))

⛔ 4 release-owned page(s) also name something this change touched. These are read-only:

content/docs/releases/v14.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/releases/v15.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/releases/v16.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))
content/docs/releases/v17.mdx (via /api/v1/auth (route, a path literal in AuthManager; a path literal in getAuthIssuer; a path literal in getMcpResourceUrl))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json db769829f856f1947de53ac6eff0927ded75364f → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1831c85fe35ef593f524cb7563a6efb1d1a6a7a9 — the merge of head e441ba11aa9b1078a7c3a549fc49e81c005e26ba into base db769829f856f1947de53ac6eff0927ded75364f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1831c85fe35ef593f524cb7563a6efb1d1a6a7a9 && git checkout 1831c85fe35ef593f524cb7563a6efb1d1a6a7a9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin db769829f856f1947de53ac6eff0927ded75364f e441ba11aa9b1078a7c3a549fc49e81c005e26ba && git checkout -B drift-repro db769829f856f1947de53ac6eff0927ded75364f && git merge --no-ff e441ba11aa9b1078a7c3a549fc49e81c005e26ba

node scripts/docs-audit/affected-docs.mjs --json db769829f856f1947de53ac6eff0927ded75364f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs db769829f856f1947de53ac6eff0927ded75364f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

claude added 2 commits September 9, 2026 00:59

Merge remote-tracking branch 'origin/main' into claude/issue-16399-ba…

e441ba1

…sepath-normaliser-consolidation

github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 9, 2026

os-trump mentioned this pull request Sep 9, 2026

[finding] AuthManager still carries two more independent basePath normalisers — getAuthIssuer() and getMcpResourceUrl() — and one of them builds a malformed URL #16399

Closed

os-trump marked this pull request as ready for review September 9, 2026 01:30

os-trump enabled auto-merge September 9, 2026 01:30

yinlianghui mentioned this pull request Sep 9, 2026

[PM seat] domain:services — 🟢 os-trump · session_012zTkyNHJ7TkuN2oXtP5x37 · R25 · (consolidated-seat takeover of 09-09T01:4xZ withdrawn 06:3xZ — see brief) #6021

Open

os-trump added this pull request to the merge queue Sep 9, 2026

os-trump mentioned this pull request Sep 9, 2026

The #3424 admin override has no Console control: a record stranded behind an unstaffed approval is releasable only through the API #16679

Closed

Merged via the queue into main with commit dd2fd20 Sep 9, 2026
34 of 35 checks passed

os-trump deleted the claude/issue-16399-basepath-normaliser-consolidation branch September 9, 2026 02:09

Navigation Menu

Uh oh!

fix(plugin-auth): collapse the three basePath derivations onto one normalisation chain - #17002

fix(plugin-auth): collapse the three basePath derivations onto one normalisation chain#17002
os-trump merged 2 commits into
mainfrom
claude/issue-16399-basepath-normaliser-consolidation

os-trump commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

os-trump commented Sep 9, 2026

Anchors, re-derived by text (⛔ not inherited)

Defect 1 — the value was not a URL, and nothing downstream tolerated it

Defect 2 — ⛔ the premise is falsified; this PR pins that it stays that way

The single normaliser, and the sites that read it

What moves, and what provably does not

Verification

Acceptance notes

What stays open on #16399

Uh oh!

github-actions Bot commented Sep 9, 2026

📓 Docs Drift Check

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants